Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/tasks/pollContentRunTask.ts`:
- Around line 109-127: The fetch to callbackUrl in pollContentRunTask.ts has no
timeout; wrap the request in an AbortController and start a timer (e.g., 5–15s)
that calls controller.abort() after the timeout, pass signal to fetch, and clear
the timer after the response arrives; handle AbortError in the surrounding
try/catch so the task continues to return success (log a timeout-specific
message), and ensure the existing logger.error branch still records non-timeout
HTTP failures for the response variable.

---

Nitpick comments:
In `@src/schemas/pollContentRunSchema.ts`:
- Line 4: The runIds array currently enforces at least one element but allows
empty strings; update the schema in pollContentRunSchema.ts so each element is a
non-empty string by changing the element schema from z.string() to
z.string().min(1) (or z.string().nonempty()), i.e., use
z.array(z.string().min(1)).min(1) on the runIds field to reject empty run IDs
while still requiring at least one item.

In `@src/tasks/pollContentRunTask.ts`:
- Line 90: Remove the unused local variable allCompleted declared in
pollContentRunTask (the const allCompleted = results.every(r => r.status ===
"completed"); line) since it isn’t referenced in the status determination logic;
delete that declaration and ensure any logic that determines overall status
continues to rely on the existing used variables (e.g., results, anyCompleted,
anyFailed) without replacing or duplicating behavior.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/tasks/pollContentRunTask.ts`:
- Around line 5-6: The polling currently uses MAX_ATTEMPTS and
POLL_INTERVAL_SECONDS which only bound iterations and can exceed the intended
max duration because runs.retrieve() adds variable latency; change to a
time-based timeout by adding a MAX_DURATION_SECONDS (or maxDurationSeconds) and
capture a start timestamp at the top of pollContentRunTask, then in the polling
loop compute elapsed = now - start and break when elapsed >= maxDurationSeconds
(instead of checking MAX_ATTEMPTS). Update logic that previously relied on
MAX_ATTEMPTS (and any per-iteration post-loop handling) to mark remaining runs
as timed out and send the callback when the elapsed timeout is reached; keep
using POLL_INTERVAL_SECONDS for sleep between iterations but ensure the final
sleep is bounded by remaining time (maxDurationSeconds - elapsed) so the task
never waits past the timeout. Ensure references updated around
POLL_INTERVAL_SECONDS, MAX_ATTEMPTS, runs.retrieve(), and the main
pollContentRunTask loop.
- Around line 74-76: The catch currently logs retrieval errors for a runId but
leaves runId in pendingRunIds so transient or permanent retrieval failures are
later misreported as "timeout"; add a retrieval failure counter (e.g., a map
retrievalFailureCounts keyed by runId and a constant RETRIEVAL_FAILURE_THRESHOLD
like 3), increment it in the catch that surrounds logger.error("Error retrieving
run", { runId, error }), and when the counter reaches the threshold remove the
runId from pendingRunIds and immediately mark/emit the run as failed (invoke the
same failure path you use elsewhere — e.g., call the existing failure handler or
emit status "failed" for that runId) while logging the final failure; reset the
counter to 0 on successful retrievals so only consecutive retrieval errors
trigger the forced failure.
- Around line 89-100: The current logic in pollContentRunTask.ts collapses mixed
outcomes to "completed" because overallStatus is set to "completed" whenever any
result has status "completed"; change the decision to use the allCompleted
boolean: keep the timeout precedence (check anyTimeout first), then set
overallStatus = "completed" only when allCompleted is true, otherwise set
overallStatus = "failed" when not allCompleted (and not timed out). Update the
block that assigns overallStatus (referencing results, anyTimeout, anyCompleted,
allCompleted, and overallStatus) so a mixed {completed, failed} array does not
report as fully "completed".
- Around line 104-127: The callback currently can be sent with an empty
x-callback-secret and non-2xx responses are only logged (so pollContentRunTask
treats failure as success); update the callback logic in pollContentRunTask to
(1) validate that callbackSecret (process.env.CODING_AGENT_CALLBACK_SECRET)
exists and if not, log an error and throw or return a failure immediately, (2)
wrap the fetch to catch network exceptions and treat them as failures, and (3)
on non-ok responses from the fetch (response.ok === false) throw or return a
failure instead of only logging—use the existing logger for context
(callbackUrl, status, body) and ensure the task bubbles the error so the caller
sees the failed handoff.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/content/sendContentCallback.ts`:
- Around line 28-31: The code reads the wrong environment variable name: change
the lookup of process.env.CODING_AGENT_CALLBACK_SECRET to
process.env.CONTENT_AGENT_CALLBACK_SECRET and update the thrown error message
accordingly (the const callbackSecret in sendContentCallback.ts should use
CONTENT_AGENT_CALLBACK_SECRET and the Error text should match) so the runtime
expects the PR-specified env var.

---

Nitpick comments:
In `@src/content/sendContentCallback.ts`:
- Around line 8-17: Extract the resolveOverallStatus function into its own
module (e.g., resolveOverallStatus.ts) so the file containing
sendContentCallback exports only that one function; move the implementation of
resolveOverallStatus out, export it from the new module (named export), update
sendContentCallback to import { resolveOverallStatus } from the new module, and
update any tests or other files that reference resolveOverallStatus to import
from the new module as well.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/content/__tests__/pollContentRuns.test.ts`:
- Around line 93-109: The test "resets retrieval failure count on successful
retrieve" currently only throws two retrieval errors so a broken cumulative
counter could still pass; update the mockRetrieve implementation used in this
test (mockRetrieve and its callCount logic inside the test for pollContentRuns)
to throw a third non-consecutive error before returning the final COMPLETED
result (e.g., add another conditional like if (callCount === 5) throw new
Error("Network error") and shift the final return accordingly) so that the test
fails unless the failure counter is actually reset between successful retrieves.

---

Nitpick comments:
In `@src/content/__tests__/sendContentCallback.test.ts`:
- Around line 31-45: The test currently hardcodes the production callback URL;
update it to set process.env.RECOUP_API_BASE_URL (or the env var your code
reads) in the test setup and assert that sendContentCallback constructs the URL
from that base. Specifically, in
src/content/__tests__/sendContentCallback.test.ts, before calling
sendContentCallback, assign a test base (e.g., "https://test-base/") to
RECOUP_API_BASE_URL, call sendContentCallback("thread-1", "completed", results),
then assert that the fetch call used
`${process.env.RECOUP_API_BASE_URL}/api/content-agent/callback` (referencing
sendContentCallback) instead of the hardcoded domain; restore or clear the env
var after the test.